Changelog verification#2580
Conversation
anannya03
requested review from
KarishmaGhiya,
g2vinay,
jairmyree,
msalaman,
saikoumudi,
vukelich and
wbreza
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds CI enforcement for changelog hygiene by requiring either a changelog entry in the PR diff or an explicit skip-changelog PR label, including GitHub authentication in PR pipelines to enable label checks.
Changes:
- Introduces
Test-ChangelogEntry.ps1to detect changelog entry files and/or validateskip-changelogvia GitHub REST API. - Hooks the new validation into
Analyze-Code.ps1and fails analysis when missing both entry and label. - Updates the analyze pipeline job to authenticate to GitHub for PR repos.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| eng/scripts/Test-ChangelogEntry.ps1 | Adds the changelog/label validation script using git diff + GitHub API label lookup |
| eng/scripts/Analyze-Code.ps1 | Executes changelog validation and turns failures into analysis errors |
| eng/pipelines/templates/jobs/analyze.yml | Adds GitHub login step for PR repos so label checks can authenticate |
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Two bugs that'll prevent this from working correctly:
-
parameters.Repositorydoesn't exist inanalyze.yml- the condition on line 12 will always evaluate to false, so the GitHub login step never runs.common.ymlcalls this template without passing aRepositoryparameter. -
Regex misses
.yamlfiles inTest-ChangelogEntry.ps1- the pattern only matches.ymlbut this repo has both.ymland.yamlchangelog entries.
Also: 8 commits with debug messages ('trial 1', 'trial 2', etc.) - please squash before merge.
github-project-automation
Bot
moved this from Untriaged
to In Progress
in Azure MCP Server
anannya03
force-pushed
the
changelog_verify
branch
from
1d24b9f to
9141365
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Two concerns with the changelog validation script:
-
In PR validation pipelines where
Build.Repository.Nameends with-pr,BUILD_REPOSITORY_NAMEcarries that suffix into the GitHub API URL (e.g.,microsoft/mcp-pr). That repo doesn't exist on GitHub, so the API call would 404. The login-to-github step runs correctly in that case, but the repo path needs the suffix stripped. -
The label fetch (lines 11-31) runs before the file check (lines 37-49). If the API call fails, the build fails hard even when valid changelog entries exist. Reordering to check files first would make this more resilient since most PRs will have changelog entries and won't need the label check at all.
Side note: @jongio's earlier CHANGES_REQUESTED (against an older commit) has been addressed. The pipeline condition now uses variables['Build.Repository.Name'] and the regex matches both .yml and .yaml.
| if ($prNumber) { | ||
| $repo = $env:BUILD_REPOSITORY_NAME | ||
| if (-not $repo) { | ||
| $repo = "microsoft/mcp" |
There was a problem hiding this comment.
If Build.Repository.Name is microsoft/mcp-pr in PR validation pipelines (which the endsWith(..., '-pr') condition in analyze.yml is designed to detect), this variable would be microsoft/mcp-pr and the API call on line 17 would hit a non-existent GitHub repo.
Consider stripping the suffix:
| $repo = "microsoft/mcp" | |
| $repo = $env:BUILD_REPOSITORY_NAME | |
| if (-not $repo) { | |
| $repo = "microsoft/mcp" | |
| } | |
| $repo = $repo -replace '-pr$', '' |
| # Fetch PR labels once at the start via GitHub REST API | ||
| $prNumber = $env:SYSTEM_PULLREQUEST_PULLREQUESTNUMBER | ||
| $prLabels = @() | ||
| if ($prNumber) { |
There was a problem hiding this comment.
Consider checking for changelog files before fetching labels. Most PRs that include a changelog entry don't need the label check at all, and this way the build won't break if the API call fails for any reason (auth issue, rate limit, wrong repo name):
# Check changed files first
Push-Location $RepoRoot
$changedFiles = git diff --name-only --diff-filter=AM origin/main...HEAD
$hasChangelog = $changedFiles | Where-Object { $_ -match 'changelog-entries/.*\.ya?ml$' -or $_ -match 'CHANGELOG\.md' }
if ($hasChangelog) {
Write-Host "Found changelog entry"
exit 0
}
Pop-Location
# Only fetch labels if no changelog files found
if ($prNumber) {
# ... label fetch logic
}
What does this PR do?
Adds changelog verification so PRs must either include a changelog entry or carry the
skip-changeloglabel.This change updates the Build pipeline to authenticate to GitHub for PR repos, invokes a new
Test-ChangelogEntry.ps1validation step fromAnalyze-Code.ps1, and adds the new script that checks changed files for changelog entries and falls back to validating the PR label through the GitHub REST API.GitHub issue number?
#2495
Pre-merge Checklist
servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationREADME.mdchanges running the script./eng/scripts/Process-PackageReadMe.ps1. See Package READMEToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.jsonbreaking-changelabelservers/Azure.Mcp.Server/docs/azmcp-commands.md./eng/scripts/Update-AzCommandsMetadata.ps1to update tool metadata inazmcp-commands.md(required for CI)servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline